Skip to content

Add $state.getAll() #411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 13, 2013
Merged

Add $state.getAll() #411

merged 3 commits into from
Sep 13, 2013

Conversation

eahutchins
Copy link

$state.get() is insufficient for introspecting the entire state table, getAll() returns the entire table in one call (useful for debugging lazy-loaded states, for example).

@nateabele
Copy link
Contributor

Could this just be the behavior of $state.get() when arguments.length === 0?

@eahutchins
Copy link
Author

I think it would make more sense for $state.get() to return the implicit root state (though '' works for that). getAll() is more of a debug/development API and probably not useful in production, which is why I kept it separate (I toyed with populating a children array on parent states but that seems more fragile than it's worth).

@nateabele
Copy link
Contributor

I can actually think of a few valid use cases for using this functionality in production (and @stu-salsbury can & has argued for this and other tree-traversal methods at length), and I really don't see a problem with passing '' to get the root state. Most importantly though, the API design of having get() vs. getAll() feels distinctly un-Angular to me.

@eahutchins
Copy link
Author

I've got no strong objection to get() as the API, I'll change it.

@laurelnaiad
Copy link

Indeed anything to chip away at the gaps from detour makes me warm and fuzzy... tree traversal, json support... I'm feeling fuzzy now but that could be a coffee OD.

That said, I might know from where @eahutchins is coming. Since the states and urlRouter routes aren't internally stored as trees it felt to me as if I was operating in some sort of fragile parallel universe given that I was going to be modifying definitions at runtime.. That conclusion was the moment when I blew away my fork and after testing the waters with getting ui-router onto a tree created detour.

That said, if step one is to have the api support tree-style traversal and tree-based storage (and editing) needs to wait (perhaps forever) it still moves in a direction I like.

@nateabele
Copy link
Contributor

I think we could probably move forward with another accessor like children() which gets all the immediate (or optionally recursive) children of a state.

@timkindberg
Copy link
Contributor

👍

nateabele added a commit that referenced this pull request Sep 13, 2013
@nateabele nateabele merged commit 69818c9 into angular-ui:master Sep 13, 2013
@nateabele
Copy link
Contributor

@timkindberg Don't forget to update the docs to reflect get()'s new behavior.

@timkindberg
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants